[FSSDK-11513] limit number of events in the eventStore #1053
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR limits the number of events stored in the event store to prevent excessive storage use. Key changes include:
- Adding a new log message (EVENT_STORE_FULL) for cases when the event store reaches capacity.
- Implementing event count management via eventCountInStore and a dedicated storeEvent method with logic to skip storing events when the maximum is reached.
- Updating tests to cover scenarios when the event store limit is met and when store saves fail.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/message/log_message.ts | Added new log message constant for full event store notification. |
| lib/event_processor/batch_event_processor.ts | Introduced event store capacity logic (eventCountInStore, storeEvent) and replaced dispatchingEventIds with dispatchingEvents. |
| lib/event_processor/batch_event_processor.spec.ts | Added tests covering event store limits and failure conditions. |
| this.eventStore?.remove(id); | ||
| events.forEach((event) => { | ||
| this.eventStore?.remove(event.id); | ||
| if (!event.notStored && this.eventCountInStore) { |
There was a problem hiding this comment.
The check 'this.eventCountInStore' may fail when the count is 0 (since 0 is falsy), which could prevent the intended decrement. Consider replacing it with 'this.eventCountInStore !== undefined' to ensure the condition works correctly even when the count is 0.
| if (!event.notStored && this.eventCountInStore) { | |
| if (!event.notStored && this.eventCountInStore !== undefined) { |
Copilot uses AI. Check for mistakes.
| @@ -138,7 +146,8 @@ export class BatchEventProcessor extends BaseService implements EventProcessor { | |||
| (currentBatch.length > 0 && !areEventContextsEqual(currentBatch[0].event, event.event))) { | |||
| batches.push({ | |||
| request: buildLogEvent(currentBatch.map((e) => e.event)), | |||
| ids: currentBatch.map((e) => e.id), | |||
| // ids: currentBatch.map((e) => e.id), | |||
There was a problem hiding this comment.
[nitpick] If the 'ids' array is no longer needed after switching to 'events', please consider removing the commented code to improve code clarity.
| // ids: currentBatch.map((e) => e.id), |
Copilot uses AI. Check for mistakes.
raju-opti
force-pushed
the
raju/event_store_limit
branch
from
d3ea562 to
67d0c18
Compare
Summary
Test plan
Issues